Open
Conversation
5 tasks
cerati
approved these changes
Jan 16, 2026
Contributor
cerati
left a comment
cerati
left a comment
There was a problem hiding this comment.
Looks great to me, thanks!
kjplows
moved this from Open pull requests
to Partially reviewed
in SBN software development
Contributor
|
Sorry to add to the bump spam @PetrilloAtWork, could you please take a look at this? Thanks! |
PetrilloAtWork
requested changes
May 7, 2026
Member
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Standard guideline changes requested.
My apologies for the inane time it took me to turn my attention to this 15' review work.
Comment on lines
+50
to
+53
| <version ClassVersion="15" checksum="2711257956"/> | ||
| <version ClassVersion="14" checksum="809065913"/> | ||
| <version ClassVersion="13" checksum="290910659"/> | ||
| <version ClassVersion="12" checksum="434873519"/> |
Member
There was a problem hiding this comment.
Remove the transient versions from the final commit.
Comment on lines
+81
to
+85
| <class name="caf::SRSlice" ClassVersion="26"> | ||
| <version ClassVersion="26" checksum="2614728424"/> | ||
| <version ClassVersion="25" checksum="3061622568"/> | ||
| <version ClassVersion="24" checksum="2614728424"/> | ||
| <version ClassVersion="23" checksum="3732772050"/> |
Member
There was a problem hiding this comment.
Same.
Suggested change
| <class name="caf::SRSlice" ClassVersion="26"> | |
| <version ClassVersion="26" checksum="2614728424"/> | |
| <version ClassVersion="25" checksum="3061622568"/> | |
| <version ClassVersion="24" checksum="2614728424"/> | |
| <version ClassVersion="23" checksum="3732772050"/> | |
| <class name="caf::SRSlice" ClassVersion="23"> | |
| <version ClassVersion="23" checksum="2614728424"/> |
Member
There was a problem hiding this comment.
Initialise the variables inline in the class definition rather than using a constructor. This .cxx will not be needed any more (or it will be empty).
Comment on lines
+20
to
+23
| class SRNuGraphSliceInfo { | ||
| public: | ||
|
|
||
| SRNuGraphSliceInfo(); |
Member
| * @brief Information on the slice by NuGraph. | ||
| * | ||
| * This object summarizes the results from running NuGraph over hits in a slice | ||
| * (see e.g. [https://sbn-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=40585](SBN DocDB 40585). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds some plane-by-plane slice-level variables to the CAFs, based on the predictions from NuGraph2.
Specifically:
shr_hitsfor theShowercategory);ng_vtx_hip_hits: the number of "HIP" hits tagged by NuGraph2 near the reconstructed interaction vertex. Wire and tick allowed distances are configurable. Standard is 10 wires and 50 ticks;unclustered_shr_hits: the number of hits tagged as "shower" by NuGraph2 in the slice, that are not associated to any reconstructed PFP.Associated PRs
Checklist
git fetchand pulled the latest changes from the branch you're basing your PR against? Will do, after review.ClassVersionby one compared to develop in classes_def.xml?